Skip to content

feat(shortestpath): Map of Alacrity (League 6) webwalker support#1750

Merged
chsami merged 7 commits intochsami:developmentfrom
runsonmypc:feature/league-6-map-of-alacrity
Apr 19, 2026
Merged

feat(shortestpath): Map of Alacrity (League 6) webwalker support#1750
chsami merged 7 commits intochsami:developmentfrom
runsonmypc:feature/league-6-map-of-alacrity

Conversation

@runsonmypc
Copy link
Copy Markdown
Contributor

Summary

Wires the League 6 tier-3 relic Map of Alacrity (item 33233) into the shortestpath pathfinder + walker so WebWalker can use its ~124 agility-shortcut teleports across 10 regions.

  • Teleports live in seasonal_transports.tsv and are varbit-gated by LEAGUE_TYPE > 0 (League worlds only).
  • Reuses the existing SEASONAL_TRANSPORT pipeline and the useSeasonalTransports toggle — no new transport type.
  • Widget 187 two-step picker (region → destination) is handled via in-game hotkeys (19 then AZ) rather than clickWidget, which reliably bypasses the scroll-fold problem that blocks click on off-screen rows.
  • Destination matching is token-based and tolerant of colour tags, punctuation, and inner colons so TSV names without colons (e.g. Chaos Temple Stepping Stone) still resolve to in-game labels that have them (Chaos Temple: Stepping Stone).
  • Session caches for locked regions and locked rows so the pathfinder doesn't keep re-probing doomed routes.

Testing — what's been manually verified

I only have a subset of League relic unlocks available, so only these regions were empirically validated in-game by running every accessible MoA teleport and comparing landing-vs-expected:

  • Kandarin (full)
  • Karamja (accessible rows)
  • Varlamore (full)
  • Wilderness (accessible rows)

46 destinations were visited. 38 TSV coords were corrected to match observed landing tiles — including one plane-mismatch (Corsair cove: Cave pillar jump was on plane 0, actually lands on plane 1) and one pair of entries whose coords were swapped between rows (Wilderness Slayer Cave JelliesLesser Demons).

Testing — what has NOT been verified in-game

The following regions were locked on my account and their coords are still sourced from wiki/community data only:

  • Asgarnia
  • Desert
  • Fremennik
  • Kourend
  • Morytania
  • Tirannwn

They load cleanly and the pathfinder routes through them, but their landing tiles are not empirically confirmed. Coord corrections for these are expected as a follow-up — see the audit plugin below.

Debug tooling included

plugins/microbot/moaaudit/MoaAuditPlugin.java — disabled-by-default one-shot plugin. On enable, iterates every accessible MoA transport, presses the teleport, and logs LAND EXACT|close|NEAR|FAR | actual=X,Y,Z expected=X,Y,Z dist=N | <label>. Any contributor with additional League unlocks can enable it and PR coord fixes for the unverified regions.

Test plan

  • ./gradlew :runelite-client:compileJava compiles
  • On a League world with Map of Alacrity in inventory, enable the Shortest Path plugin and toggle Use Seasonal Transports on
  • Set a WebWalker destination that the pathfinder should solve via MoA (e.g. within Kandarin), confirm the walker triggers the teleport and continues pathing from the landing tile
  • Walker handles locked regions/destinations cleanly (logged + blacklisted, no infinite loop)
  • (Optional) Enable the MoA Audit plugin and verify the landing audit runs to completion for your accessible regions

Adds 116 Map of Alacrity destination rows to seasonal_transports.tsv,
gated by VarbitID.LEAGUE_TYPE > 0 so they only appear on League worlds.
Covers 10 regions (Asgarnia, Desert, Fremennik, Kandarin, Karamja,
Kourend, Morytania, Tirannwn, Varlamore, Wilderness).

Reuses existing SEASONAL_TRANSPORT wiring + useSeasonalTransports
toggle -- no new Java code. Updates the toggle description to name
Map of Alacrity (Clue compass removed from description since the
relic no longer exists; existing Clue compass rows left intact as
dead edges, not our code to delete).
- TransportType.isTeleport() now treats SEASONAL_TRANSPORT as a
  teleport so null-origin MoA rows pass the dispatcher filter.
- Rs2Walker: handleSeasonalTransport() opens item 33233, finds the
  matching row in widget 187.3 by text/name/action (full region-name
  first, shortcut-name fallback), clicks. dumpMapOfAlacrityWidget()
  logs the first-page layout so we can confirm the real in-game
  label format. Dispatcher branch wired in handleTransports().
- seasonal_transports.tsv: header renamed Items -> Item IDs (parser
  expected the latter; was silently dropping the column). All 116
  MoA rows now require item 33233, so the pathfinder uses them only
  when the relic is on you. Legacy Clue compass "30363=1" values
  normalized to plain "30363" to keep the parser happy.
- PathfinderConfig: loud [MoA] INFO logging on refresh + per-row
  rejection reasons (feature flag, varbit, missing items).

Not yet validated in-game — base is 171 commits behind upstream,
rebase pending.
Walker handler for MoA widget interaction:
- Two-step widget flow (region picker → destination picker, widget 187.3)
- Thread-safe widget access via runOnClientThreadOptional
- Detects <str>...</str> strikethrough for locked regions/destinations
- Session blacklist for failed destinations to break pathfinder retry loops

PathfinderConfig + CollisionMap:
- [MoA] tracing logs (refreshTransports kept/seen, getNeighbors,
  useTransport rejection reasons) to debug pathfinder selection
- TransportType.SEASONAL_TRANSPORT gets isTeleport()=true so null-origin
  rows pass the teleport dispatcher filter

seasonal_transports.tsv:
- Display names aligned 1:1 with the in-game widget text (regions use
  full Leagues labels like "Kharidian Desert"; destinations use exact
  casing/punctuation per widget dump)
- Added 7 missing Kandarin rows: Barbarian outpost: Basalt causeway,
  Catherby: Water obelisk grapple, and all 5 additional Yanille
  destinations (Wall climb + 4 dungeon variants)
- Coord corrections (validated via AgilityShortcut enum and
  agility_shortcuts.tsv origin rows; MoA lands at the less-favorable
  / start side of each shortcut):
    Kandarin Coal trucks: Log balance
    Asgarnia Dwarf Mine Crevice
    Asgarnia Falador Crumbling Wall
    Asgarnia God Wars Dungeon Rocky Handholds
    Asgarnia Taverley Wall Climb
    Asgarnia Taverley Dungeon Loose Railing / Blue Dragon Pipe / Floor Trap
    Fremennik Slayer Dungeon Traps
- Wilderness level bumped to 60 on MoA rows (no in-game wildy cap)
- Header "Items" renamed to "Item IDs" to match the parser's field key
… log noise

Map of Alacrity Nemus rows corrected to the actual in-game teleport
landing tiles (empirically verified):
  north: 1367 3328
  south: 1370 3293
  east:  1391 3309

agility_shortcuts.tsv: the Nemus east "Broken wall" rows were
registered under ObjectID 56996 (AV_LOWWALL_CLIMB_1 — the two south
walls), same ID as the south walls. Corrected to 56997
(AV_LOWWALL_CLIMB_2) to match AgilityShortcut.java. Without this the
walker, after an east-MoA teleport, would call
Rs2GameObject.interact(56996, "Climb-over") and resolve the nearest
matching object — which was the south wall at (1387, 3303) rather
than the east wall at (1389, 3309), sending the bot on a huge detour.

Also in this pass:
  - Kandarin Coal trucks: Log balance flipped to 2598 3475 (enum origin)
  - 8 coord fixes from the region-audit agents (Asgarnia Taverley /
    Dwarf Mine / Falador Crumbling Wall / God Wars Rocky Handholds,
    Fremennik Slayer Dungeon Traps)
  - Zanaris: Cosmic altar path duplicate row removed (game widget only
    shows one entry; the medium-Agility coord is retained)

Logging cleanup: [MoA] progress/trace lines demoted log.info -> log.debug
in PathfinderConfig.refreshTransports, CollisionMap.getNeighbors, and
most of the Rs2Walker handler steps. Two entry/exit lines
(handleSeasonalTransport "entry" + "clicking destination") left at
log.info for on-going calibration of new MoA landing tiles — will be
demoted once the data is stable.
Destinations below the visible fold in the Map of Alacrity widget
could not be selected because clickWidget fails on off-screen rows,
and rows whose in-game label has an inner colon (e.g. "Chaos Temple:
Stepping Stone") never matched the TSV short name ("Chaos Temple
Stepping Stone") under substring search.

- findMoaDestinationWidget: token-contains match that normalises
  punctuation / <col>/<str> tags / case on both sides, so TSV rows
  without inner colons still locate the correct widget child.
- Selection now presses the row's keybind via Rs2Keyboard.keyPress —
  works regardless of scroll position. Primary source parses "[X]"
  or "X:" from the row text; fallback computes 1-9 then A-Z from
  position among non-locked siblings. Click path kept as last-ditch.
Handler now selects both the region and the destination via in-game
hotkey (1-9 then A-Z) rather than clickWidget, so rows below the
scroll fold are reachable. Widget matching is tokenised and tolerant
of colour tags / punctuation / colons, which unblocks destinations
whose in-game label carries an inner ":" that the TSV name doesn't
(e.g. "Chaos Temple: Stepping Stone"). Locked regions and individual
locked rows are cached per-session so the pathfinder stops re-probing
them.

Adds a disabled-by-default debug plugin (microbot/moaaudit) that
iterates every accessible MoA teleport, records landing-vs-expected
coords, and buckets the diff (EXACT/close/NEAR/FAR). Used this to
empirically correct 38 destination coords in seasonal_transports.tsv
(Kandarin, Karamja, Varlamore, Wilderness), including one plane fix
and one pair of swapped entries (Wilderness Slayer Cave Jellies /
Lesser Demons).
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 19, 2026

Walkthrough

This pull request introduces support for seasonal transports, specifically "Map of Alacrity," across the pathfinding and walker systems. Changes include a new MoaAuditPlugin debug plugin that runs a background worker thread, updates to TransportType to classify seasonal transports as teleports, additions to PathfinderConfig and CollisionMap to track and filter MoA transports with debug logging, and comprehensive handling in Rs2Walker including transport dispatch logic, session-level destination blacklisting, locked-region tracking, and a public audit routine for testing seasonal transport behavior.

Possibly related PRs


Note: There may be a potential race condition worth reviewing. The MoaAuditPlugin spawns a daemon thread that executes Rs2Walker.runMoaAudit(), which clears the session caches (blacklistedMoaDestinations and lockedMoaRegions) at startup. However, PathfinderConfig.useTransport() checks these caches to reject blacklisted MoA destinations during pathfinding. If the background audit thread runs concurrently with active pathfinding, clearing these caches could cause the pathfinder to re-attempt locked or unresolvable destinations.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.10% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: adding League 6 Map of Alacrity teleport support to the shortestpath webwalker system.
Description check ✅ Passed The description comprehensively covers the changeset, explaining Map of Alacrity integration, testing details, known limitations, and debug tooling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/CollisionMap.java`:
- Around line 157-162: The debug log after processing MoA neighbors misreports
the TransportNode cost by hardcoding "+ 4" instead of using each transport's
actual duration; update getNeighbors so it accumulates or records the real
per-neighbor cost(s) (e.g., compute cost =
config.getDistanceBeforeUsingTeleport() + transport.getDuration() when creating
each TransportNode) and then include those actual costs in the debug message
(replace the hardcoded "+ 4" with the tracked duration values or a summary of
the per-neighbor costs) so moaSeenHere/moaAddedHere/moaVisited/moaIgnored
logging reflects the true TransportNode cost(s).

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java`:
- Around line 2596-2600: In closeMoaWidgetIfOpen(), after sending the ESC via
Rs2Keyboard.keyPress(27) do not use the fixed sleep(400); instead wait until the
MoA widget actually disappears by replacing the static sleep with a conditional
wait such as calling sleepUntil(...) that repeatedly checks
Rs2Widget.isWidgetVisible(MAP_OF_ALACRITY_WIDGET_GROUP,
MAP_OF_ALACRITY_LIST_CHILD) has become false (with a sensible timeout, e.g.
1–2s) so the method only proceeds once the widget is closed or the wait times
out.
- Around line 2410-2424: The matching in findMoaWidget uses substring checks
(hay.contains(t)) causing false positives; change it to token-equality
membership: split normaliseMoaText(w.getText()) into tokens (e.g., hayTokens via
.split("\\s+")), build a Set (or list) of those tokens, and for each token t
from tokens ensure the set contains t (skip empty tokens) before accepting the
Widget; this preserves use of collectMoaChildren and normaliseMoaText but
replaces substring matching with exact token membership.
- Around line 2298-2379: When blacklisting a destination
(blacklistedMoaDestinations.add(packedDest)) the current path remains unchanged
and the MoA picker widget can stay open, so update each branch that adds to
blacklistedMoaDestinations (the branches after the early locked-region check,
after region not found, after dest never appeared, and after dest locked) to
also (1) force a path recalculation on the walker (call the walker/pathfinder
recalculation method used in this class — e.g. recalculatePath() or
refreshRoute() — so the current route is invalidated) and (2) close the Map of
Alacrity picker before returning (use the widget-close mechanism available in
this codebase such as Rs2Widget.closeWidget(MAP_OF_ALACRITY_WIDGET_GROUP) or
sending an Escape via Rs2Keyboard, ensuring the widget is no longer visible via
Rs2Widget.isWidgetVisible), then return; also keep the existing
dumpMapOfAlacrityWidget(...) logging where useful.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ee265f21-3069-4c77-aff4-d7676457b837

📥 Commits

Reviewing files that changed from the base of the PR and between 3f86a31 and ee92f4e.

⛔ Files ignored due to path filters (2)
  • runelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/agility_shortcuts.tsv is excluded by !**/*.tsv
  • runelite-client/src/main/resources/net/runelite/client/plugins/microbot/shortestpath/seasonal_transports.tsv is excluded by !**/*.tsv
📒 Files selected for processing (6)
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/moaaudit/MoaAuditPlugin.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathConfig.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/TransportType.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/CollisionMap.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java

Comment on lines +157 to +162
if (moaSeenHere > 0) {
log.debug("[MoA] getNeighbors @ ({},{},{}): seen={} added={} visited={} ignored={} (distanceBeforeUsingTeleport={}, cost={})",
x, y, z, moaSeenHere, moaAddedHere, moaVisited, moaIgnored,
config.getDistanceBeforeUsingTeleport(),
config.getDistanceBeforeUsingTeleport() + 4);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Logged cost diverges from the actual TransportNode cost.

At line 149 the cost added for a MoA teleport neighbor is config.getDistanceBeforeUsingTeleport() + transport.getDuration(), but this debug line logs config.getDistanceBeforeUsingTeleport() + 4 — a hardcoded 4 instead of transport.getDuration(). If any MoA TSV row has a duration ≠ 4, the "cost" field in this log will silently misreport what the pathfinder actually used, which defeats the purpose of the audit log. It's also a single summary value for a loop that may have added multiple MoA transports with different durations.

🔧 Suggested fix: track the real per-neighbor cost during the loop
         int moaSeenHere = 0;
         int moaAddedHere = 0;
         int moaVisited = 0;
         int moaIgnored = 0;
+        int moaLastCost = -1;
@@
             if (TransportType.isTeleport(transport.getType())) {
                 if (config.isIgnoreTeleportAndItems()) {
                     if (isMoa) moaIgnored++;
                     continue;
                 }
-                neighbors.add(new TransportNode(transport.getDestination(), node, config.getDistanceBeforeUsingTeleport() + transport.getDuration()));
-                if (isMoa) moaAddedHere++;
+                int teleCost = config.getDistanceBeforeUsingTeleport() + transport.getDuration();
+                neighbors.add(new TransportNode(transport.getDestination(), node, teleCost));
+                if (isMoa) { moaAddedHere++; moaLastCost = teleCost; }
             } else {
                 neighbors.add(new TransportNode(transport.getDestination(), node, transport.getDuration()));
             }
@@
         if (moaSeenHere > 0) {
-            log.debug("[MoA] getNeighbors @ ({},{},{}): seen={} added={} visited={} ignored={} (distanceBeforeUsingTeleport={}, cost={})",
-                    x, y, z, moaSeenHere, moaAddedHere, moaVisited, moaIgnored,
-                    config.getDistanceBeforeUsingTeleport(),
-                    config.getDistanceBeforeUsingTeleport() + 4);
+            log.debug("[MoA] getNeighbors @ ({},{},{}): seen={} added={} visited={} ignored={} (distanceBeforeUsingTeleport={}, lastCost={})",
+                    x, y, z, moaSeenHere, moaAddedHere, moaVisited, moaIgnored,
+                    config.getDistanceBeforeUsingTeleport(), moaLastCost);
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/CollisionMap.java`
around lines 157 - 162, The debug log after processing MoA neighbors misreports
the TransportNode cost by hardcoding "+ 4" instead of using each transport's
actual duration; update getNeighbors so it accumulates or records the real
per-neighbor cost(s) (e.g., compute cost =
config.getDistanceBeforeUsingTeleport() + transport.getDuration() when creating
each TransportNode) and then include those actual costs in the debug message
(replace the hardcoded "+ 4" with the tracked duration values or a summary of
the per-neighbor costs) so moaSeenHere/moaAddedHere/moaVisited/moaIgnored
logging reflects the true TransportNode cost(s).

Comment on lines +2298 to +2379
if (lockedMoaRegions.contains(region.toLowerCase())) {
log.debug("[MoA] region '{}' already known-locked — skipping '{}'", region, shortName);
blacklistedMoaDestinations.add(packedDest);
return false;
}

String action = relic.getAction("Read");
if (action == null) action = relic.getActionFromList(Arrays.asList("Read", "Open", "Teleport", "Invoke"));
if (action == null) {
log.warn("[MoA] no usable action; available={}", Arrays.toString(relic.getInventoryActions()));
return false;
}
if (!Rs2Inventory.interact(relic, action)) {
log.warn("[MoA] Rs2Inventory.interact returned false for action '{}'", action);
return false;
}

// Step 1: wait for the region picker to render, then click the matching region.
if (!sleepUntil(() -> Rs2Widget.isWidgetVisible(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD), 3000)) {
log.warn("[MoA] region widget {}.{} did not open", MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD);
return false;
}

Widget regionRoot = Rs2Widget.getWidget(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD);
if (regionRoot == null) {
log.warn("[MoA] region widget lookup returned null");
return false;
}
dumpMapOfAlacrityWidget(regionRoot);

Widget regionMatch = findMoaWidget(regionRoot, region);
if (regionMatch == null) {
log.warn("[MoA] region '{}' not found in picker — check dump", region);
return false;
}
// Locked regions render with <str>...</str> strikethrough markup. Don't waste a press.
String regionText = Microbot.getClientThread().runOnClientThreadOptional(regionMatch::getText).orElse("");
if (regionText != null && regionText.contains(MOA_LOCKED_MARKUP)) {
log.warn("[MoA] region '{}' is locked (text='{}') — caching + blacklisting destination {}",
region, regionText, transport.getDestination());
lockedMoaRegions.add(region.toLowerCase());
blacklistedMoaDestinations.add(packedDest);
return false;
}
log.debug("[MoA] selecting region '{}'", region);
Character regionHotkey = extractMoaHotkey(regionText);
if (regionHotkey == null) regionHotkey = computeMoaHotkeyByIndex(regionRoot, regionMatch);
if (regionHotkey != null) {
Rs2Keyboard.keyPress(regionHotkey);
} else {
log.warn("[MoA] no hotkey resolved for region '{}' — falling back to clickWidget", region);
if (!Rs2Widget.clickWidget(regionMatch)) {
log.warn("[MoA] region click returned false");
return false;
}
}

// Step 2: wait for the destination to appear in the (same) widget. If the region was
// locked or otherwise non-clickable, this poll will time out with shortName never
// showing, and we return false.
Widget destMatch = sleepUntilNotNull(() -> {
Widget root = Rs2Widget.getWidget(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD);
if (root == null) return null;
return findMoaWidget(root, shortName);
}, 3000);

if (destMatch == null) {
log.warn("[MoA] destination '{}' never appeared after clicking region '{}' — name mismatch or locked; blacklisting",
shortName, region);
Widget root = Rs2Widget.getWidget(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD);
if (root != null) dumpMapOfAlacrityWidget(root);
blacklistedMoaDestinations.add(packedDest);
return false;
}

// Individual destinations can also be locked inside an unlocked region.
String destText = Microbot.getClientThread().runOnClientThreadOptional(destMatch::getText).orElse("");
if (destText != null && destText.contains(MOA_LOCKED_MARKUP)) {
log.warn("[MoA] destination '{}' is locked (text='{}') — blacklisting", shortName, destText);
blacklistedMoaDestinations.add(packedDest);
return false;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Recalculate and close the MoA picker when blacklisting a failed route.

These branches update the session blacklist, but the current path was already built, so the walker can retry the same stale MoA edge until some later recalculation happens. The widget also stays open on several failure paths.

🐛 Proposed fix
+    private static boolean abortMoaAttempt(int packedDest, boolean blacklistDestination) {
+        if (blacklistDestination && blacklistedMoaDestinations.add(packedDest) && currentTarget != null) {
+            recalculatePath();
+        }
+        closeMoaWidgetIfOpen();
+        return false;
+    }
+
         if (lockedMoaRegions.contains(region.toLowerCase())) {
             log.debug("[MoA] region '{}' already known-locked — skipping '{}'", region, shortName);
-            blacklistedMoaDestinations.add(packedDest);
-            return false;
+            return abortMoaAttempt(packedDest, true);
         }
@@
         Widget regionMatch = findMoaWidget(regionRoot, region);
         if (regionMatch == null) {
             log.warn("[MoA] region '{}' not found in picker — check dump", region);
-            return false;
+            return abortMoaAttempt(packedDest, true);
         }
@@
             log.warn("[MoA] region '{}' is locked (text='{}') — caching + blacklisting destination {}",
                     region, regionText, transport.getDestination());
             lockedMoaRegions.add(region.toLowerCase());
-            blacklistedMoaDestinations.add(packedDest);
-            return false;
+            return abortMoaAttempt(packedDest, true);
         }
@@
             log.warn("[MoA] no hotkey resolved for region '{}' — falling back to clickWidget", region);
             if (!Rs2Widget.clickWidget(regionMatch)) {
                 log.warn("[MoA] region click returned false");
-                return false;
+                return abortMoaAttempt(packedDest, false);
             }
         }
@@
             Widget root = Rs2Widget.getWidget(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD);
             if (root != null) dumpMapOfAlacrityWidget(root);
-            blacklistedMoaDestinations.add(packedDest);
-            return false;
+            return abortMoaAttempt(packedDest, true);
         }
@@
         if (destText != null && destText.contains(MOA_LOCKED_MARKUP)) {
             log.warn("[MoA] destination '{}' is locked (text='{}') — blacklisting", shortName, destText);
-            blacklistedMoaDestinations.add(packedDest);
-            return false;
+            return abortMoaAttempt(packedDest, true);
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java`
around lines 2298 - 2379, When blacklisting a destination
(blacklistedMoaDestinations.add(packedDest)) the current path remains unchanged
and the MoA picker widget can stay open, so update each branch that adds to
blacklistedMoaDestinations (the branches after the early locked-region check,
after region not found, after dest never appeared, and after dest locked) to
also (1) force a path recalculation on the walker (call the walker/pathfinder
recalculation method used in this class — e.g. recalculatePath() or
refreshRoute() — so the current route is invalidated) and (2) close the Map of
Alacrity picker before returning (use the widget-close mechanism available in
this codebase such as Rs2Widget.closeWidget(MAP_OF_ALACRITY_WIDGET_GROUP) or
sending an Escape via Rs2Keyboard, ensuring the widget is no longer visible via
Rs2Widget.isWidgetVisible), then return; also keep the existing
dumpMapOfAlacrityWidget(...) logging where useful.

Comment on lines +2410 to +2424
private static Widget findMoaWidget(Widget root, String shortName) {
String normalised = normaliseMoaText(shortName);
if (normalised.isEmpty()) return null;
String[] tokens = normalised.split(" ");
return Microbot.getClientThread().runOnClientThreadOptional(() -> {
for (Widget w : collectMoaChildren(root)) {
String hay = normaliseMoaText(w.getText());
if (hay.isEmpty()) continue;
boolean all = true;
for (String t : tokens) {
if (t.isEmpty()) continue;
if (!hay.contains(t)) { all = false; break; }
}
if (all) return w;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use exact token membership instead of substring matching.

This is documented as token-based matching, but hay.contains(t) matches substrings, so a shorter token can match inside an unrelated row and select the wrong MoA destination.

🐛 Proposed fix
     private static Widget findMoaWidget(Widget root, String shortName) {
         String normalised = normaliseMoaText(shortName);
         if (normalised.isEmpty()) return null;
-        String[] tokens = normalised.split(" ");
+        Set<String> tokens = Arrays.stream(normalised.split(" "))
+                .filter(t -> !t.isEmpty())
+                .collect(Collectors.toSet());
         return Microbot.getClientThread().runOnClientThreadOptional(() -> {
             for (Widget w : collectMoaChildren(root)) {
                 String hay = normaliseMoaText(w.getText());
                 if (hay.isEmpty()) continue;
-                boolean all = true;
-                for (String t : tokens) {
-                    if (t.isEmpty()) continue;
-                    if (!hay.contains(t)) { all = false; break; }
-                }
+                Set<String> hayTokens = Arrays.stream(hay.split(" "))
+                        .filter(t -> !t.isEmpty())
+                        .collect(Collectors.toSet());
+                boolean all = tokens.stream().allMatch(hayTokens::contains);
                 if (all) return w;
             }
             return null;
         }).orElse(null);
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java`
around lines 2410 - 2424, The matching in findMoaWidget uses substring checks
(hay.contains(t)) causing false positives; change it to token-equality
membership: split normaliseMoaText(w.getText()) into tokens (e.g., hayTokens via
.split("\\s+")), build a Set (or list) of those tokens, and for each token t
from tokens ensure the set contains t (skip empty tokens) before accepting the
Widget; this preserves use of collectMoaChildren and normaliseMoaText but
replaces substring matching with exact token membership.

Comment on lines +2596 to +2600
private static void closeMoaWidgetIfOpen() {
if (Rs2Widget.isWidgetVisible(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD)) {
Rs2Keyboard.keyPress(27); // ESC
sleep(400);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Wait for the MoA widget to actually close.

A fixed sleep(400) can continue with the picker still open after ESC; verify the widget state instead.

🐛 Proposed fix
     private static void closeMoaWidgetIfOpen() {
         if (Rs2Widget.isWidgetVisible(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD)) {
             Rs2Keyboard.keyPress(27); // ESC
-            sleep(400);
+            sleepUntil(() -> !Rs2Widget.isWidgetVisible(
+                    MAP_OF_ALACRITY_WIDGET_GROUP,
+                    MAP_OF_ALACRITY_LIST_CHILD), 3000);
         }
     }

As per coding guidelines, “Never use static sleeps like sleep(12000) to wait for game state. Always use conditional dynamic sleeps: sleepUntil(BooleanSupplier awaitedCondition).”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static void closeMoaWidgetIfOpen() {
if (Rs2Widget.isWidgetVisible(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD)) {
Rs2Keyboard.keyPress(27); // ESC
sleep(400);
}
private static void closeMoaWidgetIfOpen() {
if (Rs2Widget.isWidgetVisible(MAP_OF_ALACRITY_WIDGET_GROUP, MAP_OF_ALACRITY_LIST_CHILD)) {
Rs2Keyboard.keyPress(27); // ESC
sleepUntil(() -> !Rs2Widget.isWidgetVisible(
MAP_OF_ALACRITY_WIDGET_GROUP,
MAP_OF_ALACRITY_LIST_CHILD), 3000);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java`
around lines 2596 - 2600, In closeMoaWidgetIfOpen(), after sending the ESC via
Rs2Keyboard.keyPress(27) do not use the fixed sleep(400); instead wait until the
MoA widget actually disappears by replacing the static sleep with a conditional
wait such as calling sleepUntil(...) that repeatedly checks
Rs2Widget.isWidgetVisible(MAP_OF_ALACRITY_WIDGET_GROUP,
MAP_OF_ALACRITY_LIST_CHILD) has become false (with a sensible timeout, e.g.
1–2s) so the method only proceeds once the widget is closed or the wait times
out.

@chsami chsami merged commit 7bcfde9 into chsami:development Apr 19, 2026
1 check failed
runsonmypc added a commit to runsonmypc/Microbot that referenced this pull request Apr 19, 2026
MoaAuditPlugin and the runMoaAudit()/closeMoaWidgetIfOpen() helpers in
Rs2Walker were intended as a temporary offline landing-coord audit tool
(self-labelled [TEMP] "Delete when done") and were supposed to be held
on a separate debug branch, not merged. They slipped into upstream with
PR chsami#1750 because the amend that removed them locally was never pushed
before the PR was merged.

Scope: delete-only. No functional changes to MoA handling —
handleSeasonalTransport, lockedMoaRegions, and blacklistedMoaDestinations
are untouched.
runsonmypc added a commit to runsonmypc/Microbot that referenced this pull request Apr 19, 2026
MoaAuditPlugin and the runMoaAudit()/closeMoaWidgetIfOpen() helpers in
Rs2Walker were intended as a temporary offline landing-coord audit tool
(self-labelled [TEMP] "Delete when done") and were supposed to be held
on a separate debug branch, not merged. They slipped into upstream with
PR chsami#1750 because the amend that removed them locally was never pushed
before the PR was merged.

Scope: delete-only. No functional changes to MoA handling —
handleSeasonalTransport, lockedMoaRegions, and blacklistedMoaDestinations
are untouched.
runsonmypc added a commit to runsonmypc/Microbot that referenced this pull request Apr 19, 2026
MoaAuditPlugin and the runMoaAudit()/closeMoaWidgetIfOpen() helpers in
Rs2Walker were intended as a temporary offline landing-coord audit tool
(self-labelled [TEMP] "Delete when done") and were supposed to be held
on a separate debug branch, not merged. They slipped into upstream with
PR chsami#1750 because the amend that removed them locally was never pushed
before the PR was merged.

Scope: delete-only. No functional changes to MoA handling —
handleSeasonalTransport, lockedMoaRegions, and blacklistedMoaDestinations
are untouched.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants